Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial support for RISC-V 32 I M #781

Open
wants to merge 121 commits into
base: main
Choose a base branch
from
Open

Initial support for RISC-V 32 I M #781

wants to merge 121 commits into from

Conversation

clebreto
Copy link
Contributor

@clebreto clebreto commented Apr 8, 2024

Fixes #698 and #503 and #783.

rtetley and others added 27 commits February 5, 2024 09:14
…s (ADD / ADDI). (#706)

* Provides RISCV initial architecture description

and first instructions (ADD / ADDI).

Co-authored-by: Benjamin Gregoire <[email protected]>
Co-authored-by: Jean-Christophe Léchenet <[email protected]>

---------

Co-authored-by: Benjamin Gregoire <[email protected]>
Co-authored-by: Jean-Christophe Léchenet <[email protected]>
Co-authored-by: LE BRETON Come <[email protected]>
Co-authored-by: LE BRETON Come <[email protected]>
Co-authored-by: Santiago Arranz Olmos <[email protected]>
Add XOR and OR to compiler/riscv_instr_decl.v

---------

Co-authored-by: Julian Wälde <[email protected]>
Co-authored-by: Jean-Christophe Léchenet <[email protected]>
Renames registers.

Co-authored-by: Jean-Christophe Léchenet <[email protected]>
Co-authored-by: Jean-Christophe Léchenet <[email protected]>
Co-authored-by: Jean-Christophe Léchenet <[email protected]>
Co-authored-by: Jean-Christophe Léchenet <[email protected]>
Co-authored-by: Jean-Christophe Léchenet<[email protected]>
Co-authored-by: Jean-Christophe Léchenet
<[email protected]>
Co-authored-by: Jean-Christophe Léchenet
<[email protected]>
+ Fix push / pop ra to stack
It looks like gp may be used to access globals. I don't know for tp, but
to be safe I added both of them into the list of callee-saved registers.
eponier and others added 7 commits September 16, 2024 18:12
This pass compiles, on RISC-V, complex loads and stores into
3 equivalent instructions for which assembly can be generated.

This is kind of a hack for now. Ideally, we would support subarrays with
non-constant offset and a way to do pointer arithmetic.
for arm.

Proof still required.

Co-authored-by: Jean-Christophe Léchenet
<[email protected]>
Proof missing in params_core.

Co-authored-by: Jean-Christophe Léchenet <[email protected]>
…ctions-in-a-same-fashion-as-it-was-done-for-arm-1

789 risc v test every instructions in a same fashion as it was done for arm 1

Bug fixes to main risc-v branch have been added, as well as compiler pass.

It notably adds a lowering compiler pass for complex addresses.
@clebreto clebreto marked this pull request as ready for review September 18, 2024 08:03
Copy link
Member

@vbgl vbgl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this hard work. My comments are mostly cosmetic.

@@ -17,7 +17,7 @@ let show_intrinsics asmOp fmt =
let headers = [|
"no size suffix";
"one optional size suffix, e.g., “_64”";
"a zero/sign extend suffix, e.g., “_u32u16”";
"a zero/sign extend suffix, e.g., “_s16” or “_u32u16”";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is surprising: does it mean that it is now possible to write MOVSX_s16?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any updates on this issue?

Comment on lines +67 to +74
ppxlib
ppx_import
ppx_sexp_conv
ppx_yojson_conv
ppx_deriving
sel
lsp
sexplib
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change really meant to be included in this PR?

Comment on lines 239 to 241
(*CHECKME*)
Definition riscv_sra_semi (wn : ty_r) (wm : word U8) : exec ty_r := ok (wsar wn (wunsigned (wand wm (wrepr U8 31)))).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be checked before merging?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 132 to 133
(* TODO: move *)
Lemma write_lval_eq_ex wdb gd X x v s1 s2 vm1 :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be addressed before merging.

Comment on lines +271 to +272
(* TODO: move *)
Lemma sem_sopn_eq_ex X gd o xs es s1 s2 vm1 :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

riscv_decl
riscv_extra
riscv_instr_decl
(* riscv_instr_decl_lemmas *)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this comment be removed?

Definition riscv_loparams : lowering_params lowering_options :=
{|
lop_lower_i _ _ _ := lower_i;
lop_fvars_correct := fun _ _ _ => true; (* FIXME RISCV: is this correct? *)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix this.

Comment on lines 110 to 111
(* FIXME try to remove the usage of this lemma, use sem_fopn_args version instead *)
Lemma align_eval_instr {lp ls ii xname vi y al} {wy : word Uptr} :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really a FIXME? If it is the case, then please fix it. Otherwise please change to TODO.

Comment on lines 124 to 125
(* FIXME try to remove the usage of this lemma, use sem_fopn_args version instead *)
Lemma sub_eval_instr {lp ls ii xname vi y z} {wy wz : word Uptr} :
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Comment on lines 268 to 269
(* Lemma li_lsem_1 s xname vi imm :
let: (xi, x) := mkv xname vi in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment might be removed.

clebreto and others added 9 commits September 30, 2024 16:22
The passes between stack_alloc and reg_alloc are called twice, first in
OCaml and then in Coq. Pass LowerAddressing was inserted between
stack_alloc and reg_alloc, but was only called in Coq, creating a
discrepancy between the two calls (in OCaml and in Coq). The pass is now
called also in OCaml.

Due to the way complex Coq types are extracted in OCaml, we had to make
some types simpler on the Coq side to be able to call the pass in OCaml.
RA is not used as the return address storage AND the return value in
the same function.
bgregoir and others added 8 commits October 17, 2024 13:53
RISC-V: automatically introduce registers for constants in conditions

This is implemented as a new arch-independent pass called before lowering.
A boolean in arch_params controls whether the pass is called (true for RISC-V,
false otherwise).

Co-authored-by: Jean-Christophe Léchenet <[email protected]>
On RISC-V, the return address is read from register RA. This was not
reflected in the model and generated wrong programs.

The RAstack case of return_address_location is given an additional
optional argument ra_return that specifies what register to use (if any)
when returning from a function. linearization is adapted to generate the
right code. reg alloc and merge_varmaps take into account this potential
new register.
Problem : jasmin2ec not working for armm4 or riscv -> x86 used instead
in the header.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Basic description of RISC-V32 I + M + ZIFENCEI ISA
7 participants